Skip to content

new: add map_threads option for ThreadPool size in map pipe - #320

Merged
theseal merged 1 commit into
IdentityPython:masterfrom
REANNZ:add-map-threads-opt
Oct 20, 2026
Merged

theseal merged 1 commit into
IdentityPython:masterfrom
REANNZ:add-map-threads-opt

Conversation

@vladimir-mencl-eresearch

@vladimir-mencl-eresearch vladimir-mencl-eresearch commented May 25, 2026 •

Copy link
Copy Markdown
Contributor

Allow configuring ThreadPool size used in map pipe.

Passing default value None makes ThreadPool use os.process_cpu_count(), keeping existing behaviour.

Setting the value to 1 provides a workaround for #315 (avoid parallelism).

Given that most processing happens withing Python without releasing GIL, it actually does not incur any significant penalty.

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what problem you are trying to solve with this PR?
  • Have you added information on what your changes do and why you chose this as your solution?
  • Have you written new tests for your changes?
  • Does your submission pass tests?
  • This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

@vladimir-mencl-eresearch

Copy link
Copy Markdown
Contributor Author

Rebased on current master

Allow configuring ThreadPool size used in map pipe.

Passing default value None makes ThreadPool use os.process_cpu_count(),
keeping existing behaviour.

Setting the value to 1 provides a workaround for IdentityPython#315 (avoid parallelism).

Given that most processing happens withing Python without releasing GIL,
it actually does not incur any significant penalty.
@vladimir-mencl-eresearch

Copy link
Copy Markdown
Contributor Author

@theseal rebased with signed commits

@vladimir-mencl-eresearch

Copy link
Copy Markdown
Contributor Author

This PR should not be needed after #337 is merged.

@theseal

theseal commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Do you still agree with your earlier statement that this PR is not longer needed since change to pyuppsala?

@vladimir-mencl-eresearch

Copy link
Copy Markdown
Contributor Author

Do you still agree with your earlier statement that this PR is not longer needed since change to pyuppsala?

No longer needed as workaround for the race condition reported in #315 .

However, it might still be useful if a deployment wanted to tweak the parallelism instead of going with Python default of one worker thread per discovered CPU core.

Up to you whether you see it useful for the project.

@theseal
theseal merged commit 20700c1 into IdentityPython:master Oct 20, 2026
13 checks passed
@theseal

theseal commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Don't see any harm letting user configured their pyff in different ways. Thanks! 🙂

@vladimir-mencl-eresearch
vladimir-mencl-eresearch deleted the add-map-threads-opt branch October 20, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants